Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Prettier log output #221

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MarcusMogg
Copy link

feat:Prettier log output with log src file and line, looks like:

image

test pr

@hyv1001
Copy link
Collaborator

hyv1001 commented Jun 4, 2022

Thanks very much for this PR!
It's a neat feature to print the filename/line/function while logging and is very useful in certain situations.
However, for engine development, we prefer to keep the flexibility of each function. For example, maybe the developers don't want to output the above info to keep the simplicity of the logs, or some projects just need the log file's size as small as possible.
Therefore, we recommend implementing this feature in individual projects as needed, rather than in the engine.

@ameaninglessname
Copy link

Could this become a alternative log macro rather than the default one?

@hyv1001
Copy link
Collaborator

hyv1001 commented Jun 4, 2022

Could this become a alternative log macro rather than the default one?

I think it's OK, but no idea about a proper naming.

@@ -20,29 +20,47 @@ namespace Pilot
fatal
};

static constexpr const char* FileName(const char* path)
Copy link
Contributor

@ShenMian ShenMian Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 该方法相比于 C++ 更适合 C, C 使用 strrchr(), C++ 可以考虑使用 std::string_view.
  2. 这种判断方法不适用于处理 Windows 的路径, 比如在 MSVC 中宏 __FILE__ 的值.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 这里其实用到了c++14 起的一个特性,constexpr 函数的编译期计算,可以看这里 https://godbolt.org/z/jjYqfjz7s , O1级别以上的优化都会在编译期算出表达式,也就是执行期不会call function
    image
  2. 没有理解
  3. 确实,只做了简单的测试,用 std::filesystem::path::preferred_separator替代即可

Copy link
Contributor

@ShenMian ShenMian Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没注意到 constexpr, 抱歉. strrchr() 确实不支持, 不过 std::string_view 需要使用到的成员函数应该支持.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static constexpr std::string_view FileName(std::string_view path)
{
    return path.substr(path.find_last_of(std::filesystem::path::preferred_separator) + 1);
}

仅供参考.

@MarcusMogg
Copy link
Author

Could this become a alternative log macro rather than the default one?

Maybe make log pattern configurable

@MarcusMogg
Copy link
Author

feat:

  1. 使用Pilot自带的JSON序列化功能实现配置读取,替换原有的ini,简化代码,方便增加配置项
  2. 令log pattern可配置,具体配置内容见 pattern-flags
    @hyv1001 @ShenMian @ameaninglessname

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants